Skip to content

Remove pick tuning panel from teleop cockpit - #555

Closed
theo-michel wants to merge 22 commits into
mainfrom
remove-pick-panel-583ed7
Closed

Remove pick tuning panel from teleop cockpit#555
theo-michel wants to merge 22 commits into
mainfrom
remove-pick-panel-583ed7

Conversation

@theo-michel

Copy link
Copy Markdown
Contributor

Summary

  • Delete webapp/js/teleop/pickPanel.js — the per-stage slider panel (wrench button, o shortcut) for live-tuning pick_any_object params; we no longer want to expose this option
  • Unwire it from the teleop cockpit in main.js
  • Remove its CSS section (.picktune-panel*) from app.css

Left intact on purpose: the pick overlay (pickOverlay.js) and its .picktune-* CSS, the PICK_DEBUG_TOPIC/PICK_TUNING_TOPIC constants, and the skill-side tuning handler — the overlay's draggable wrist box still uses them.

Test plan

  • grep -rn 'pickPanel\|picktune-panel\|createPickPanel' webapp/ returns nothing
  • Load the teleop page: cockpit renders without the wrench button; o no longer toggles anything; pick overlay still works

theo-michel and others added 22 commits July 15, 2026 07:02
…ing panel

Add a wrist-camera fine-alignment stage to the pick_any_object grasp, and the
Teleop tuning panel that drives it. After the base parks the object in the
head-camera pick box, the arm goes to an operator-posed search position (up
high, elbow ~90°, wrist camera looking down), Gemini marks the object, and the
arm visual-servos straight down onto it: center-in-box first, then descend one
step, repeating until a stop height where the blind ladder finishes the grasp.

Tracking is HSV color segmentation + CamShift, not optical flow: during the
descent the object grows ~2.5x in the wrist image and fabric deforms, which
slides LK patches onto the carpet. A likelihood-ratio color model (object vs
surrounding floor) back-projected each frame is scale- and deformation-proof,
and its score honestly reports occlusion/loss instead of tracking carpet.

The pick_any_object skill and its runtime helpers (arm_rest_position,
gripper_open, gripper_close) move from the gitignored workspace/custom_skills/
into workspace/innate_skills/ so the pipeline is tracked and works from a fresh
checkout. Bare-name skill chaining still resolves them (local/ then innate-os/).

Panel (Teleop, key "o"): live top-down + side grasp views, per-stage sliders
that publish overrides the running skill applies mid-run, and a draggable wrist
box + detection marker over the arm-camera video for aiming the servo.

Topics: skill -> /pick_any_object/debug (stage events), panel ->
/pick_any_object/tuning (parameter overrides).
Introduce workspace/skill_lib/ — plain Python modules skills import directly,
no Skill classes, no roster entries. Functions take the interfaces they need
as explicit arguments. The hardware lessons now live exactly once:

- skill_lib/arm.py: verified gripper open (overcurrent-trip reboot recovery),
  close with the strength-cap lesson, health-checked cartesian moves (FK
  verify, recover + retry, raise ArmUnhealthy), servo recovery, reach clamps.
- skill_lib/geometry.py: head-camera <-> floor pinhole projection, moved
  verbatim from pick_any_object (pure math; the webapp panel mirrors it).

gripper_open / gripper_close become thin wrappers over the lib — and
gripper_open GAINS the trip-recovery verification that previously only
pick_any_object had. pick_any_object drops ~150 lines of private helpers
(_move_arm, _recover_arm, geometry, inline reach clamps) for lib calls, and
calls the lib directly instead of round-tripping gripper skills through the
invoker.

Import contract: skills import the lib at module top — the loader puts the
repo root on sys.path only while a skill module executes (namespace-package
import, verified live). catalog.reload_all/reload_selective now evict cached
workspace.skill_lib modules so a normal skills reload picks up lib edits too.

No behavior or compatibility changes otherwise: skill IDs, innate.skills
chaining, panel debug events, and TUNABLE keys are unchanged. Verified live
on the robot: reload loads all 23 skills, pick_any_object acks its params
through the lib imports, and gripper_close/gripper_open ran end-to-end via
the skill CLI (claw physically cycled and returned to open).
refactor(skills): shared skill_lib for arm primitives + camera geometry
Replace the direct google-genai client in pick_any_object with the Innate
ProxyClient. Vision requests now go to the proxy's OpenAI-compatible endpoint
(service "gemini", POST /v1/chat/completions) with the image as a data URI —
the proxy holds the upstream API key; the robot authenticates with its
service key. Drops the GEMINI_API_KEY load path and the genai dependency from
this skill.

Verified against the live proxy that the service name, endpoint, and vision
body shape are all accepted (they pass path validation and body parsing and
reach the proxy's auth check). The native Gemini REST path
(models/{model}:generateContent) is NOT usable through the proxy — it rejects
the ":" in the path — so OpenAI-compat is the route.

KNOWN BLOCKER (deploy step, not a code issue): this robot's service key is
not yet authorized for the "gemini" service — the proxy returns 403 until an
admin grants access. Detection will fail with 403 until then; grant the key
"gemini" access, then verify a real pick end-to-end.
feat(pick): route vision calls through the Innate proxy
…ation refactor

pick_any_object:
- _grasp_verified now sends BOTH head + wrist cameras to Gemini so a
  held object (visible in the mirrored wrist view) isn't misread as
  dropped; degrades to head-only if the wrist frame is absent.
- cancellation via SkillCancelled + _checkpoint() replaces the
  cancelled-as-None plumbing (one raise, one except in execute); fixes
  cancel-during-search being misreported as FAILURE.
- extracted the pure vision math and proxy vision call into
  workspace/skill_lib/{vision,gemini}.py; gemini.ask_image now takes
  one image or a list (multi-image support).

Also folded in (per 'everything'):
- pyright: return-type annotations on Skill base (types.py) +
  pyrightconfig.json; /etc/innate.env perms enforcement in
  post_update.sh; webapp pick tuning panel rename to pickOverlay.js;
  in-progress pyright cleanups across other innate_skills.
The pickOverlay rewrite pinned the boxes to hardcoded TUNABLE defaults;
the old panel drew them from params synced off the debug topic (and drag
published updates back). Any live-tuned value left the drawn box lying
about where the skill actually aims.

Seed a live P dict with the defaults and resync it from every run_start
and params event (the skill broadcasts its full dict on both), including
mid-run tuning acks. Non-numeric payload values are ignored.

Verified headlessly (node + DOM stubs, real module): box lands within
0.05px of Python floor_to_pixel for default and tuned params, moves on a
mid-run params ack, size follows box_half_px, hides on run_end.
…, overlay params

- post_update: rename superseded workspace/custom_skills copies (arm_rest_position,
  gripper_open, gripper_close, pick_any_object) to *.py.superseded on update, so the
  gitignored local/ copy can't shadow the now-shipped skill via catalog precedence
- ruff check + ruff format clean (CI format check); 'workspace' added to isort
  known-first-party
- pickOverlay: draw boxes from the skill's broadcast params (run_start/params debug
  events) instead of hardcoded mirrors of TUNABLE — live tuning now redraws
- drop orphaned .arm-follower CSS (the readout UI is not in this PR)
- pick_any_object: guard head interface at entry like manipulation/mobility;
  replace -O-strippable assert in _sweet_box with a raise
- catalog: document the reload_selective two-lib-copies invariant
custom_skills/ holds user-authored skills only. These four skills were
never tracked outside this PR, so no release ever shipped them under the
legacy skills/ dir and the skills/ -> custom_skills/ migration could not
have deposited them. A copy only exists where someone hand-placed one, so
the migration was dead code on every robot it would ever run on.

Dev robots used during development do have the stale copies; those get
removed directly rather than via a shipped migration.
Drop comments that narrate the code, recount how a constant was arrived at,
or duplicate a docstring that lives with the implementation. Keep the ones
stating constraints the code can't show (signed wrist gains, close_strength
trip limit, color-model-not-LK, no cancel checkpoint mid-grip).

Also repoint pickOverlay.js at skill_lib/geometry.py for the camera geometry
it mirrors — it still named pick_any_object.py, where that code no longer is.
_open_gripper_checked discarded open_checked's result, so a gripper that
stayed shut still went on to the wrist descent and close/twist/lift,
squeezing an already-tripped servo. Return the result and abort with
ArmUnhealthy instead.

open_checked could not report that failure either: after the reboot+retry
it returned the retry command's own status, and a tripped servo accepts the
command and no-ops silently — which is the whole reason the j6 check exists.
Re-verify j6 after the retry so the function keeps the promise in its
docstring. Healthy, recovers-after-reboot, and no-j6-reading paths are
unchanged.
…the skill's box

- pickPanel.js: per-stage sliders for all 45 pick_any_object TUNABLE knobs,
  toggled with 'o' (button next to profiling). Each slider publishes a one-key
  partial dict on /pick_any_object/tuning; the skill's params ack drives the
  displayed values. On open the panel publishes {} — a pure echo request — so
  it syncs to the skill's live values with no defaults mirrored in JS; sliders
  stay disabled until that first ack.
- pickOverlay.js: the head-camera pick box is now drawn from the skill's `box`
  telemetry ([cu,cv,half,accept] on run_start/params/servo/position) instead
  of reprojecting sweet_x/box_y/tilt_deg through a JS copy of the camera
  model — camPose/floorToPixel and the mirrored URDF constants are gone. The
  wrist goal box is draggable when the arm camera is primary: dropping it
  publishes wrist_box_u/v as a live tuning override (throttled ~8 Hz while
  dragging, final on release; param echoes are ignored mid-drag so the ack of
  an in-flight value can't yank the box). Marker DOM is built by one helper.
- arm_rest_position: drop the redundant __init__ — the Skill base class
  creates the cancel latch lazily and its setter ignores False.
_rest_arm(keep_grip=False) sent all-zero joints, a completely different
posture from the documented folded rest that risks driving the gripper
into the floor. REST_POSITION moves to skill_lib/arm.py (single source
of truth, hot-reloads with the lib) and both arm_rest_position and the
pick skill's fold-home now use it. A sub-skill call was not an option
here: after a cancel the invoker short-circuits child runs, so the
finally-block rest would never execute.

Also cap the close_strength slider at 0.6 — both arm.py and the TUNABLE
default agree >~0.6 overcurrent-trips the gripper servo.
The wrist-servo descent was failing on ~0.051m FK residuals against a
0.05m bar — a marginal miss the reboot recovery can't fix, aborting the
whole pick. 0.07m gives margin at the arm's stretched reach poses.
Bench testing found 0.8 grips more reliably than 0.6. Raises the
default, the panel slider cap, and updates the trip-warning comments
that previously called 0.6 the ceiling.
Move the odometry-closed rotate/drive loops and pixel P-servo out of
pick_any_object into skill_lib/mobility.py (interfaces passed explicitly,
gains as kwargs, optional cancel predicate + dbg hook), mirroring arm.py.
Also fold the arm rest loop into armlib.rest and switch move_checked to
per-axis tolerances (tol_xy/tol_z).
…/innate-os into theo/pick-wrist-servo

# Conflicts:
#	webapp/js/teleop/pickOverlay.js
#	workspace/innate_skills/arm_rest_position.py
#	workspace/innate_skills/pick_any_object.py
#	workspace/skill_lib/arm.py
…ch-pose move, strength docs

- _rest_arm(keep_grip=False) folds to armlib.REST again instead of the
  all-zero posture; the earlier fix (e407a38) was clobbered by the
  robot-side wip commit. Zero-config after a failed descent can sweep
  the gripper through the floor.
- _goto_search_pose now goes through armlib.go so a rejected search-pose
  command raises ArmFailed instead of silently corrupting the wrist
  descent from the REST position.
- gripper_close.py strength docstring updated to the live-tested ~0.8
  ceiling (was a stale <=0.6, contradicting arm.py and the tuned
  defaults). Slider max (0.8) and TUNABLE default (0.70) stay as tuned.
- pickTunePanel.js comments flagged by Greptile are moot: the panel file
  was removed in favor of pickOverlay.js.
- The unchecked-gripper-open finding was already fixed at HEAD.
Drop the slider panel (wrench button / 'o' toggle) for live-tuning
pick_any_object params — we no longer want to expose that option.
The pick overlay and the skill-side tuning topic stay, since the
overlay's draggable wrist box still publishes to it.
@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a visual pick-and-grasp workflow and shared robot-control helpers. The main changes are:

  • A new pick_any_object skill with live tuning and grasp verification.
  • Teleop overlays for head- and wrist-camera aiming.
  • Shared arm, mobility, vision, geometry, and Gemini helpers.
  • New arm-rest and gripper skills plus arm-zero refactoring.
  • Skill-library reload support, typing configuration, and update-script permission repair.

Confidence Score: 4/5

The pick workflow can remain cancelled across runs, retain stale ROS callbacks after reload, and consume unsafe tuning values.

  • A cancelled singleton can abort later executions without a new cancel request.
  • Reloads can accumulate tuning subscribers owned by retired skill instances.
  • Unchecked tuning values flow directly into timing and physical-motion calculations.

workspace/innate_skills/pick_any_object.py

Security Review

The new ROS tuning subscriber accepts unrestricted values that directly control physical-motion parameters. Publishers on the ROS graph can inject invalid gains, limits, durations, and loop bounds; each key needs finite type and safe-range validation.

Important Files Changed

Filename Overview
workspace/innate_skills/pick_any_object.py Adds the pick workflow, but run cancellation state, ROS subscription cleanup, and tuning validation can fail during normal use.
webapp/js/teleop/pickOverlay.js Adds camera-aware aim overlays, wrist-target dragging, and browser-side cleanup.
workspace/skill_lib/arm.py Adds shared arm movement, gripper, recovery, and verification helpers.
ros2_ws/src/brain/brain_client/brain_client/skills/catalog.py Evicts cached skill-library modules before full and selective skill reloads.
scripts/update/post_update.sh Normalizes ownership and permissions for the system environment file.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant UI as Teleop overlay
    participant ROS as ROS topics
    participant Current as Current pick skill
    participant Retired as Retired pick skill
    participant Robot as Robot interfaces
    UI->>ROS: Publish wrist tuning
    ROS->>Current: _on_tuning(payload)
    ROS->>Retired: Stale _on_tuning(payload)
    Current->>ROS: Params event
    Retired->>ROS: Competing params event
    Current->>Robot: Motion using live values
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant UI as Teleop overlay
    participant ROS as ROS topics
    participant Current as Current pick skill
    participant Retired as Retired pick skill
    participant Robot as Robot interfaces
    UI->>ROS: Publish wrist tuning
    ROS->>Current: _on_tuning(payload)
    ROS->>Retired: Stale _on_tuning(payload)
    Current->>ROS: Params event
    Retired->>ROS: Competing params event
    Current->>Robot: Motion using live values
Loading

Comments Outside Diff (3)

  1. workspace/innate_skills/pick_any_object.py, line 1845 (link)

    P1 Cancellation State Carries Between Runs

    Unlike the other new arm skills, this entry point never resets _cancelled. After one call to cancel(), the same singleton skill instance starts every later execution with the latch still set; the first cancellation checkpoint then aborts the new pick even though nobody cancelled that run.

  2. workspace/innate_skills/pick_any_object.py, line 1279 (link)

    P1 Reload Leaves Stale ROS Callbacks

    This subscription is created on the shared server node with a bound method, but the skill has no shutdown() cleanup. Replacing the skill during reload therefore leaves the retired instance subscribed; later tuning messages update every old instance and emit competing params events while the subscriptions and instances accumulate.

  3. workspace/innate_skills/pick_any_object.py, line 1287 (link)

    P1 security Unbounded Tuning Reaches Motion Code

    Any ROS publisher can replace motion and timing parameters with unrestricted numeric values. Inputs such as negative sleep durations, zero or negative step counts, non-finite gains, or reversed velocity bounds then flow into sleeps, loops, and physical-motion helpers, causing exceptions, skipped positioning, or incorrectly directed movement.

Reviews (1): Last reviewed commit: "Remove pick tuning panel from teleop coc..." | Re-trigger Greptile

@theo-michel

Copy link
Copy Markdown
Contributor Author

Folded into #542 (merge a7515d9c) so the whole pick + skills story lands as one PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant